Skip to content

fix(import): allow issue-only duplicate closes - #309

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/issue-only-dedupe-close
Draft

fix(import): allow issue-only duplicate closes#309
vincentkoc wants to merge 1 commit into
mainfrom
fix/issue-only-dedupe-close

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary

  • stop requiring a code fix before closing duplicates in issue-only clusters
  • preserve fix-first behavior for clusters containing pull requests
  • keep issue-only clusters quarantined when any member has a security signal

Validation

  • node --test test/import-gitcrawl-clusters.test.mjs (8/8)
  • npm run validate (6,699 jobs)

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 7, 2026, 8:28 AM ET / 12:28 UTC.

ClawSweeper review

What this changes

The PR relaxes generated fix-before-close policy for issue-only Gitcrawl clusters and adds importer fixtures for issue-only, mixed, and security-signaled clusters.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open: the PR still permits superseded issue closures without the fix path that current validation requires, and its MEMBER author prevents automatic cleanup.

Priority: P1
Reviewed head: 62039ae65b18eeff123b6781792d0745d11a47a9

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is narrowly scoped but has one concrete high-priority automation-policy defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; the reported tests remain useful supplemental validation.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; the reported tests remain useful supplemental validation.
Evidence reviewed 3 items PR policy change: The PR changes issue-only clusters to emit require_fix_before_close: false whenever they lack a security signal and pull request member, including clusters later classified as superseded.
Downstream guard distinction: Current main exempts only classification duplicate from fix-first validation; superseded closures still require an executed merge, merged candidate, or opened/pushed fix PR.
Current-main provenance: The importer and downstream validator were introduced in the current visible history by the queued-merge recording commit; the PR head is not an ancestor of current main.
Findings 1 actionable finding [P1] Keep superseded closures behind the fix-first guard
Security None None.

How this fits together

Clownfish imports Gitcrawl cluster data into guarded job files that control later automation. The generated closure policy feeds the worker and applicator that can close duplicate or superseded GitHub items.

flowchart LR
  A[Gitcrawl cluster data] --> B[Cluster importer]
  B --> C[Generated job policy]
  C --> D[Worker closure plan]
  D --> E[Closure validation]
  E --> F[Duplicate or superseded action]
Loading

Before merge

  • Keep superseded closures behind the fix-first guard (P1) - This condition emits require_fix_before_close: false for every non-security issue-only cluster. Current validation exempts only duplicate; a superseded action therefore bypasses the required fix, merge, or merged-candidate path. This is the still-unfixed blocker from the prior review cycle.
  • Resolve merge risk (P1) - Merging would let an issue-only cluster execute a superseded closure without the existing required fix, merge, or merged-candidate evidence.

Findings

  • [P1] Keep superseded closures behind the fix-first guard — scripts/import-gitcrawl-clusters.mjs:277-282
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 2 files affected; production +6/-1, tests +77/-3 A small policy edit changes the authorization boundary for automated closures.

Merge-risk options

Maintainer options:

  1. Restore the superseded guard (recommended)
    Allow the intended duplicate-only exception but keep fix-before-close enabled for superseded closures.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Restrict the issue-only exemption to duplicate closures, add a regression test that superseded closures retain the fix-first guard, and do not edit CHANGELOG.md.

Technical review

Best possible solution:

Limit the issue-only exemption to duplicate closures, retaining the fix-first guard for superseded closures and proving both outcomes with focused importer coverage.

Do we have a high-confidence way to reproduce the issue?

Yes—source inspection establishes the path: the PR disables the generated guard for an issue-only cluster, while current validation exempts only duplicate classifications, not superseded ones.

Is this the best way to solve the issue?

No—the blanket issue-only exemption is broader than the supported duplicate-only exception and bypasses the existing supersession safeguard.

Full review comments:

  • [P1] Keep superseded closures behind the fix-first guard — scripts/import-gitcrawl-clusters.mjs:277-282
    This condition emits require_fix_before_close: false for every non-security issue-only cluster. Current validation exempts only duplicate; a superseded action therefore bypasses the required fix, merge, or merged-candidate path. This is the still-unfixed blocker from the prior review cycle.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7a2042df54fe.

Labels

Label justifications:

  • P1: The branch can weaken a guard protecting active issue closure from unsupported supersession.
  • merge-risk: 🚨 automation: The diff changes generated policy consumed by automated closure workflows.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The author is a repository MEMBER, so the external-contributor real-behavior-proof gate does not apply; the reported tests remain useful supplemental validation.

Evidence

Acceptance criteria:

  • [P1] node --test test/import-gitcrawl-clusters.test.mjs.
  • [P1] npm run validate.

What I checked:

  • PR policy change: The PR changes issue-only clusters to emit require_fix_before_close: false whenever they lack a security signal and pull request member, including clusters later classified as superseded. (scripts/import-gitcrawl-clusters.mjs:277, 62039ae65b18)
  • Downstream guard distinction: Current main exempts only classification duplicate from fix-first validation; superseded closures still require an executed merge, merged candidate, or opened/pushed fix PR. (scripts/apply-result.mjs:1155, 7a2042df54fe)
  • Current-main provenance: The importer and downstream validator were introduced in the current visible history by the queued-merge recording commit; the PR head is not an ancestor of current main. (scripts/import-gitcrawl-clusters.mjs:277, 4aaf96fa0911)

Likely related people:

  • github-actions[bot]: Visible current-main history attributes both the importer and applicator guard baseline to the queued-merge recording commit. (role: introduced current automation policy; confidence: low; commits: 4aaf96fa0911; files: scripts/import-gitcrawl-clusters.mjs, scripts/apply-result.mjs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Retain the guard for superseded closures and add a focused regression test for that policy.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (64 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-06T09:15:52.888Z sha 62039ae :: needs changes before merge. :: [P1] Preserve the fix-first guard for superseded closes
  • reviewed 2026-08-07T00:59:34.659Z sha 62039ae :: needs changes before merge. :: [P1] Keep the fix-first guard for superseded closures
  • reviewed 2026-08-07T03:55:15.237Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-07T05:21:26.029Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closures behind the fix-first guard
  • reviewed 2026-08-07T05:24:59.147Z sha 62039ae :: needs changes before merge. :: [P1] Retain the fix-first guard for superseded closures
  • reviewed 2026-08-07T06:57:16.696Z sha 62039ae :: found issues before merge. :: [P1] Retain the fix-first guard for superseded closures
  • reviewed 2026-08-07T08:35:02.553Z sha 62039ae :: found issues before merge. :: [P1] Retain the fix-first guard for superseded closures
  • reviewed 2026-08-07T11:07:00.786Z sha 62039ae :: found issues before merge. :: [P1] Retain the fix-first guard for superseded closures

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 14, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant